Penalize transactions with gas above gas limit#2271
Conversation
tomusdrw
added
A0-pleasereview 🤓
|
Changes Unknown when pulling 52169b2 on tx-penalize into * on beta*. |
gavofyork
added
A8-looksgood 🦄
|
looks alright to me. |
|
Changes Unknown when pulling 745b8b6 on tx-penalize into * on beta*. |
rphmeier
left a comment
rphmeier
left a comment
There was a problem hiding this comment.
just a couple tiny code quality issues, otherwise LGTM
| self.penalties = match self.penalties.overflowing_add(1) { | ||
| (_, true) => current, | ||
| (val, false) => val, | ||
| }; |
There was a problem hiding this comment.
self.penalties = self.penalties.saturating_add(1);
| return; | ||
| } | ||
|
|
||
| let transaction = transaction.expect("Early-exit for None is above."); |
There was a problem hiding this comment.
more idiomatic to match instead of .is_none() and expect combo.
|
Changes Unknown when pulling 6fffede on tx-penalize into * on beta*. |
|
Changes Unknown when pulling d4312d1 on tx-penalize into * on beta*. |
|
Not sure about the gas before gas price change tbh -- what's the incentive under normal operating conditions? |
|
Changes Unknown when pulling d4312d1 on tx-penalize into * on beta*. |
|
to favour a regular throughput of the queue: if there is an attack underway (which there pretty much will be until we get a hard fork) then favouring small transactions:
|
gavofyork
added
A0-pleasereview 🤓
|
merging due to general positive voices and on the back of @rphmeier 's prior review |
5 participants
No description provided.